Skip to content

Conversation

mgilbir
Copy link

@mgilbir mgilbir commented Oct 6, 2025

Summary

This PR fixes critical parsing issues in the SQLite engine where comparison operators were incorrectly defaulting to = and ORDER BY clauses were generating incorrect AST nodes. It also restores correct handling of EXISTS/NOT EXISTS so SQLite queries emit the right boolean expressions and generated code.

Issues Fixed

  1. Comparison operators not parsed correctly - All comparison expressions were defaulting to = operator instead of extracting the actual operator (<, >, <=, >=, !=, <>, IS, LIKE, GLOB, MATCH, REGEXP)
  2. NOT operator variants not supported - NOT LIKE, NOT GLOB, NOT IN, IS NOT NULL, etc. were not handled properly
  3. ORDER BY generates wrong AST nodes - ORDER BY clauses were incorrectly creating CaseExpr nodes instead of proper SortBy nodes
  4. Missing ORDER BY integration - SELECT statements weren't properly setting the SortClause field
  5. NOT EXISTS flattened incorrectly - SQLite treated NOT EXISTS as a bare subquery, producing exists variables instead of the expected not_exists boolean output

Changes Made

Core Fixes (internal/engine/sqlite/convert.go)

  • Enhanced convertComparison: Now properly extracts actual comparison operators instead of defaulting to =
  • Added extractComparisonOperator: Comprehensive function handling all SQLite comparison operators including NOT variants
  • Fixed convertOrderby_stmtContext: Now generates proper SortBy AST nodes with correct direction (ASC/DESC) and null handling (NULLS FIRST/LAST)
  • Enhanced convertMultiSelect_stmtContext: Added ORDER BY integration to properly set SortClause
  • Added convertNullComparison: Handle IS NULL/IS NOT NULL expressions
  • Extended convertUnaryExpr: Detects unary NOT via the unary-operator node, including wrapping NOT EXISTS as a BoolExpr
  • Hardened convertInSelectNode: Recognises NOT EXISTS when the context begins with the NOT token so the AST preserves the negation

New Test Coverage (internal/engine/sqlite/convert_test.go)

  • 35 comprehensive test cases covering all fixed functionality
  • Comparison operator tests: All SQLite operators (<, >, <=, >=, =, !=, <>, <<, >>, &, |, IS, LIKE, GLOB, MATCH, REGEXP)
  • NOT variant tests: NOT LIKE, NOT GLOB, NOT IN, IS NOT NULL, etc.
  • ORDER BY tests: Direction handling, null ordering, multiple columns
  • Complex query tests: Real-world scenarios with multiple operators and clauses

Operators Now Supported

  • Comparison: <, >, <=, >=, =, !=, <>
  • Bitwise: <<, >>, &, |
  • Pattern matching: LIKE, GLOB, MATCH, REGEXP
  • Null testing: IS NULL, IS NOT NULL
  • List membership: IN, NOT IN
  • All NOT variants: NOT LIKE, NOT GLOB, NOT MATCH, NOT REGEXP

Testing

# All new tests pass
go test -v ./internal/engine/sqlite -run TestConvert
# Existing functionality preserved
go test ./internal/engine/sqlite
# Regression coverage for managed DB fixtures
go test ./internal/endtoend -run 'TestReplay/managed-db/select_exists/sqlite'
go test ./internal/endtoend -run 'TestReplay/managed-db/select_not_exists/sqlite'

Example Impact

Before (incorrect):

SELECT * FROM users WHERE age >= 18 ORDER BY name DESC
-- Generated AST: A_Expr with "=" operator, CaseExpr for ordering

After (correct):

SELECT * FROM users WHERE age >= 18 ORDER BY name DESC  
-- Generated AST: A_Expr with ">=" operator, SortBy with DESC direction

Breaking Changes

  • SQLite EXISTS/NOT EXISTS return types: Generated Go methods now return bool (and emit variables named exists/not_exists) instead of the previous int64 placeholder. Projects that already consume these helpers must update their call sites to expect a boolean result.
  • Parameter propagation for NOT EXISTS: Because the AST now retains the inner select, generated methods once again surface the query arguments. Call sites that relied on the buggy zero-argument signature will need to pass the correct inputs (for example, id in BarNotExists).

Related Issues

Fixes parsing issues with SQLite comparison operators and ORDER BY clauses that could lead to incorrect SQL generation and runtime errors.

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. 🔧 golang labels Oct 6, 2025
@mgilbir mgilbir marked this pull request as draft October 6, 2025 01:03
@mgilbir mgilbir marked this pull request as ready for review October 6, 2025 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files. 🔧 golang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant